Skip to content

Comments

feat(x-design-system): button component#1998

Open
Turito wants to merge 2 commits intomainfrom
feat/xds-button-component
Open

feat(x-design-system): button component#1998
Turito wants to merge 2 commits intomainfrom
feat/xds-button-component

Conversation

@Turito
Copy link
Contributor

@Turito Turito commented Feb 19, 2026

Pull request template

Describe the purpose of the change, the specific changes done in detail, and the issue you have fixed.

Motivation and context

  • Dependencies. If any, specify:
  • Open issue. If applicable, link:

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that causes existing functionality to not work as expected)
  • Change requires a documentation update

What is the destination branch of this PR?

  • Main
  • Other. Specify:

How has this been tested?

Tests performed according to testing guidelines:

Checklist:

  • My code follows the style guidelines of this project.
  • I have performed a self-review on my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.

@Turito Turito requested a review from a team as a code owner February 19, 2026 16:20
@Turito Turito force-pushed the feat/xds-button-component branch 2 times, most recently from 0353a00 to 06e69fe Compare February 20, 2026 11:26
@Turito Turito force-pushed the feat/xds-button-component branch from 06e69fe to 120c592 Compare February 20, 2026 11:28
@apply xds:flex xds:justify-center xds:items-center xds:content-center xds:flex-row xds:flex-nowrap;
@apply xds:border-1 xds:border-solid xds:cursor-pointer;
@apply xds:font-family-main xds:font-bold;
@apply xds:text-neutral-0 xds:bg-neutral-90 xds:border-neutral-90;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I miss these two properties

    boxSizing: 'border-box',
    letterSpacing: theme('x.letterSpacing.md'),

&:active {
@apply xds:bg-neutral-90 xds:border-neutral-90;
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propuse you add custom variants to achieve it.

@custom-variant selected (&.xds\:selected);
@custom-variant hover-active (&:hover,&:active);

With the first, we encapsulate our custom state selected under a single line dependency. You can use it as @variant selected.
And the second one, as it is common applying styles for both hover and active state, I think is it clearer. You can use it as @variant hover-active

}

&:disabled {
@apply xds:border-neutral-10 xds:bg-neutral-10 xds:text-neutral-25 xds:cursor-not-allowed;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave xds:cursor-not-allowed in a single @apply

&:disabled {
@apply xds:border-neutral-10 xds:bg-neutral-10 xds:text-neutral-25 xds:cursor-not-allowed;

&:hover,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This selector must be selected, mustn't it? Because it

'&.x-selected': {
    ...disabledStyles,
  },

And xds:cursor-not-allowed is not added here in the old design system

@apply xds:min-h-40 xds:gap-8 xds:ps-16 xds:pe-16 xds:text-sm;
}
@utility button-lg {
@apply xds:min-h-48 xds:gap-8 xds:ps-20 xds:pe-20 xds:text-sm;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

xds:text-md

@apply xds:min-h-0 xds:ps-0 xds:pe-0;
@apply xds:border-0 xds:underline xds:font-normal;
@apply xds:text-neutral-75 xds:bg-transparent;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing this also

'&:disabled': {
    fontWeight: 'inherit',
  },

<Wrapper feature="Button" :rows="rows">
<template #default>
<button class="xds:button">
<span>button</span>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove span

'layout',
'color',
'default-selected',
'outline',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

outlined

'color',
'default-selected',
'outline',
'outline-selected',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

outlined-selected

selected tight {{ key }} button
</button>
</template>
<template #link>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image Font weight (at least) in the demo doesn't seem the same

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants